-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change Banner layout to better align with mobile #11887
Conversation
ef65185
to
bb935f6
Compare
3f43430
to
0432570
Compare
/snapit |
🫰✨ Thanks @mauriciomeirelles! Your snapshothas been published to npm. Test the snapshot by updating your "@shopify/polaris": "0.0.0-snapshot-20240412151826" |
7be79f2
to
c2ad416
Compare
c2ad416
to
c6ff816
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code and storybook look good. Can you share the spin link when you get a chance?
'@shopify/polaris': patch | ||
--- | ||
|
||
Add border to Banner on breakpoint-xs and change success icon to CheckCircleIcon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add border to Banner on breakpoint-xs and change success icon to CheckCircleIcon | |
Added border to Banner on breakpoint-xs and change success icon to CheckCircleIcon |
@@ -3,6 +3,12 @@ | |||
position: relative; | |||
display: flex; | |||
|
|||
margin-inline: var(--p-space-300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still have to support old mobile safari versions? I thought logical properties were still out because of that. Not a blocker at all. I'm all for using them since it's not task blocking for the long tail
/snapit |
🫰✨ Thanks @mauriciomeirelles! Your snapshothas been published to npm. Test the snapshot by updating your "@shopify/polaris": "0.0.0-snapshot-20240415094702" |
7aca883
to
5c36a7b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this being applied to all versions of Banner
? Also does this change make sure their is 16px of padding between the Banner
and the edge of the screen?
5c36a7b
to
a299eb3
Compare
a299eb3
to
15930a6
Compare
15930a6
to
bf15e36
Compare
/snapit |
🫰✨ Thanks @mauriciomeirelles! Your snapshots have been published to npm. Test the snapshots by updating your "@shopify/polaris-migrator": "0.0.0-snapshot-20240429113201",
"@shopify/polaris": "0.0.0-snapshot-20240429113201",
"@shopify/polaris-tokens": "0.0.0-snapshot-20240429113201",
"@shopify/stylelint-polaris": "0.0.0-snapshot-20240429113201" |
Closing in favor of #11959 to only apply changes below |
WHY are these changes introduced?
Fixes https://github.com/Shopify/mobile/issues/33410
WHAT is this pull request doing?
Implement design from https://www.figma.com/file/PQUFqE9VrOtVPZbaifdL5J/Forms?type=design&node-id=2241-9876&mode=design&t=IEI19PTpbkZdyhKO-0
Also changes the
success
icon fromCheckIcon
toCheckCircleIcon
How to 🎩
🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines
🎩 checklist
README.md
with documentation changes